Move feature plugin package installs to Python for image mode support#4800
Move feature plugin package installs to Python for image mode support#4800
Conversation
There was a problem hiding this comment.
Code Review
This pull request transitions package installation for EPEL and FIPS features from Ansible playbooks to Python code to support image-mode guests with immutable filesystems. It also introduces distro_id and distro_major_version guest facts. Feedback suggests adding error handling for non-numeric VERSION_ID values during integer conversion to prevent potential crashes.
| if version == 9: | ||
| guest.package_manager.install(Package("epel-next-release")) | ||
|
|
||
| cls._run_playbook('enable', "epel-enable.yaml", guest, logger) |
There was a problem hiding this comment.
Is there value of keeping the remaining playbook?
There was a problem hiding this comment.
@LecrisUT yes, I do not want to implement more spagethi fun in the code only the amount is needed. Also the same for FIPS.
There was a problem hiding this comment.
Let me rephrase it. What is being done in the playbook now that there is the logic section above?
Edit: or let me put it differently, would this even do what you expect it to do since the install commands are done at build time and the playbook would run immediately without even the plugin available to handle config-manager
There was a problem hiding this comment.
Let me rephrase it. What is being done in the playbook now that there is the logic section above?
In the playbook we mostly play around with repository enablement:
Edit: or let me put it differently, would this even do what you expect it to do since the install commands are done at build time and the playbook would run immediately without even the plugin available to handle config-manager
The bootc package manager install function should install the missing dependencies to a derived container image, build the new container image and switch to it:
tmt/tmt/package_managers/bootc.py
Lines 301 to 316 in c0a3c89
So not sure I follow how it would run immediately. It runs after the missing dependencies are installed the image mode way.
There was a problem hiding this comment.
Let me rephrase it. What is being done in the playbook now that there is the logic section above?
In the playbook we mostly play around with repository enablement:
That is an answer to the litteral question, not the spirit of the question. The purpose of the question is to prod on whether it makes sense to keep that or not, which is not addressed.
Edit: or let me put it differently, would this even do what you expect it to do since the install commands are done at build time and the playbook would run immediately without even the plugin available to handle config-manager
The
bootcpackage managerinstallfunction should install the missing dependencies to a derived container image, build the new container image and switch to it:tmt/tmt/package_managers/bootc.py
Lines 301 to 316 in c0a3c89
So not sure I follow how it would run immediately. It runs after the missing dependencies are installed the image mode way.
Please read my comment again and more carefully!
There was a problem hiding this comment.
Let me rephrase it. What is being done in the playbook now that there is the logic section above?
In the playbook we mostly play around with repository enablement:
https://github.com/teemtee/tmt/blob/c0a3c8994f6be4829632b58e3c13f74dec35e0a8/tmt/steps/prepare/feature/epel-enable.yamlThat is an answer to the litteral question, not the spirit of the question. The purpose of the question is to prod on whether it makes sense to keep that or not, which is not addressed.
It does for me, I do not want to introduce more python code as necessary. The ansible code alreaedy does decent job doing what is needed except the packages installation.
Edit: or let me put it differently, would this even do what you expect it to do since the install commands are done at build time and the playbook would run immediately without even the plugin available to handle config-manager
The
bootcpackage managerinstallfunction should install the missing dependencies to a derived container image, build the new container image and switch to it:
tmt/tmt/package_managers/bootc.py
Lines 301 to 316 in c0a3c89
So not sure I follow how it would run immediately. It runs after the missing dependencies are installed the image mode way.
Please read my comment again and more carefully!
Which one, this below? Man, I read it like 10 times already :) and I thought I replied well.
Edit: or let me put it differently, would this even do what you expect it to do since the install commands are done at build time
yes, they are done at build time, but they are also applid to the live image right after each install command.
and the playbook would run immediately without even the plugin available to handle config-manager
No, the missing deps are installed and applied on the running system BEFORE the playbook runs.
I just tested it with injecting another dep, because all those ar actually alraedy baked into our images:
❯ tmt -vvv run -a plan --name '/epel/enabled/with-epel-preinstalled' provision --how virtual --image https://artifacts.dev.testing-farm.io/images/CentOS-Stream-9-image-mode-x86_64.qco
w2
...
how: feature
order: 50
> /var/home/thrix/git/github.com/teemtee/tmt/worktree/feature-plugins-image-mode/tmt/steps/prepare/feature/epel.py(76)enable()
-> import pdb; pdb.set_trace()
(Pdb) distro
'centos'
(Pdb) n
> /var/home/thrix/git/github.com/teemtee/tmt/worktree/feature-plugins-image-mode/tmt/steps/prepare/feature/epel.py(77)enable()
-> guest.package_manager.install("neovim")
(Pdb) n
cmd: rpm -q --whatprovides neovim
stdout: no package provides neovim
cmd: mktemp --directory tmp.XXXXXXXXXX
stdout: tmp.KY02SByTo0
cmd: /bin/bash -c "( ( podman pull quay.io/testing-farm/centos-bootc:stream9 || podman pull containers-storage:quay.io/testing-farm/centos-bootc:stream9 ) || bootc image copy-to-storage --target quay.io/testing-farm/centos-bootc:stream9)"
stderr: Trying to pull quay.io/testing-farm/centos-bootc:stream9...
stderr: Getting image source signatures
stderr: Copying blob sha256:9f2741e8a325eabbd2d2fdd05a403342e367cba176db17ad0cee655fd76c2620
stderr: Copying blob sha256:29b667a8b294984efb1c0aca524ff788af33ee241b4adf543a64e13e23d134ba
stderr: Copying blob sha256:0f6aaa63cc93223316d8174f136b292152af143f07236da94146f0489a38ea05
....
....
(neovim is installed before the playbook runs)
There was a problem hiding this comment.
Edit: or let me put it differently, would this even do what you expect it to do since the install commands are done at build time and the playbook would run immediately without even the plugin available to handle config-manager
...
Please read my comment again and more carefully!
Which one, this below? Man, I read it like 10 times already :) and I thought I replied well.
The conversation that I quoted. My comment there is to indicate that you have misinterpreted what I was saying, i.e. re-read what I wrote and try to find if we are misaligned between what you think I wrote, and what I was trying to write, not about your reply!
To be more precise:
- I did not say that the
installcommands are done immediately, nor will they ever be able to becausednf installfails. These are done at build time - I said that the playbooks are the things that are run immediately
- There is discrepancy between these two
yes, they are done at build time, but they are also applid to the live image right after each install command.
What? How can dnf install be applied to a live bootc image? Also be more specific on what you are referencing there, the install step, the ansible step, or something completely different. I have interpreted that you were referring to the install step.
and the playbook would run immediately without even the plugin available to handle config-manager
No, the missing deps are installed and applied on the running system BEFORE the playbook runs.
Huh, but then that means that we are doing the rebuild every time we hit an ansible step? E.g. if we have a prepare/install -> prepare/ansible -> prepare/shell, then it does: add install step to deferred commands -> build container -> run ansible -> add shell step to deferred commands -> build container? That does not fit with the understanding of what was done to support prepare/install + prepare/shell for bootc image. There is a big misalignment in how the bootc support is going on here if this is the case. Please prepare the design document more clearly in #4818 with some references in the PR so that we can all be on the same page of what is going on here.
I just tested it with injecting another dep, because all those ar actually alraedy baked into our images:
❯ tmt -vvv run -a plan --name '/epel/enabled/with-epel-preinstalled' provision --how virtual --image https://artifacts.dev.testing-farm.io/images/CentOS-Stream-9-image-mode-x86_64.qco w2 ... how: feature order: 50 > /var/home/thrix/git/github.com/teemtee/tmt/worktree/feature-plugins-image-mode/tmt/steps/prepare/feature/epel.py(76)enable() -> import pdb; pdb.set_trace() (Pdb) distro 'centos' (Pdb) n > /var/home/thrix/git/github.com/teemtee/tmt/worktree/feature-plugins-image-mode/tmt/steps/prepare/feature/epel.py(77)enable() -> guest.package_manager.install("neovim") (Pdb) n cmd: rpm -q --whatprovides neovim stdout: no package provides neovim cmd: mktemp --directory tmp.XXXXXXXXXX stdout: tmp.KY02SByTo0 cmd: /bin/bash -c "( ( podman pull quay.io/testing-farm/centos-bootc:stream9 || podman pull containers-storage:quay.io/testing-farm/centos-bootc:stream9 ) || bootc image copy-to-storage --target quay.io/testing-farm/centos-bootc:stream9)" stderr: Trying to pull quay.io/testing-farm/centos-bootc:stream9... stderr: Getting image source signatures stderr: Copying blob sha256:9f2741e8a325eabbd2d2fdd05a403342e367cba176db17ad0cee655fd76c2620 stderr: Copying blob sha256:29b667a8b294984efb1c0aca524ff788af33ee241b4adf543a64e13e23d134ba stderr: Copying blob sha256:0f6aaa63cc93223316d8174f136b292152af143f07236da94146f0489a38ea05 .... .... (neovim is installed before the playbook runs)
Please provide a simple tmt plan to properly follow. I do not follow what you showed there
feef056 to
b9094a5
Compare
Remove standalone `tests/prepare/bootc/` and extend existing plugin tests (`prepare/install`, `prepare/shell`, `prepare/recommend`) with image mode coverage on CentOS Stream 10 and Fedora 44 bootc images. - `enable_copr()` — installs copr plugin via Containerfile (since `/usr` is read-only), then runs `dnf copr enable` on the live system to write `.repo` to `/etc`. - `create_repository()`, `install_repository()` — delegate to `aux_engine` to run on the live system. Not directly tested, will be exercised when `prepare/artifact` tests are extended. Existing tests gain `IS_IMAGE_MODE` handling and run against image mode images when triggered by the new `/plans/provision/virtual/image-mode` sub-plan. Reboot persistence is verified through a dedicated `reboot-persistence` inner plan that installs packages, reboots via `tmt-reboot`, and re-verifies. A single reboot test is sufficient — all install variants (repo, local RPM, COPR, etc.) go through the same bootc mechanism (Containerfile rebuild → `bootc switch` → reboot), so if packages survive a reboot in one case, they survive in all. - `prepare/artifact`, `prepare/require` — currently `provision-container` only - `prepare/ansible`, `prepare/feature/epel,fips,profile` — need Ansible support in image mode (#4636) - `prepare/distgit` — test refactoring needed Pull Request Checklist * [x] implement the feature * [x] extend the test coverage Assisted-by: Claude Code Signed-off-by: Miroslav Vadkerti <mvadkert@redhat.com>
Signed-off-by: Miroslav Vadkerti <mvadkert@redhat.com>
Signed-off-by: Miroslav Vadkerti <mvadkert@redhat.com>
Signed-off-by: Miroslav Vadkerti <mvadkert@redhat.com>
This reverts commit 05d59b0.
Signed-off-by: Miroslav Vadkerti <mvadkert@redhat.com>
Co-authored-by: Cristian Le <git@lecris.dev>
Signed-off-by: Miroslav Vadkerti <mvadkert@redhat.com>
Signed-off-by: Miroslav Vadkerti <mvadkert@redhat.com>
Script from remote url is broken, see #4785 Signed-off-by: Miroslav Vadkerti <mvadkert@redhat.com>
Signed-off-by: Miroslav Vadkerti <mvadkert@redhat.com>
Signed-off-by: Miroslav Vadkerti <mvadkert@redhat.com>
Signed-off-by: Miroslav Vadkerti <mvadkert@redhat.com>
Signed-off-by: Miroslav Vadkerti <mvadkert@redhat.com>
Signed-off-by: Miroslav Vadkerti <mvadkert@redhat.com>
The `image` and `in_subdirectory` variables in `fetch_downloaded_packages()` were not declared `local`, so when `IMAGE_MODE=yes` the function overwrote the global loop `$image` (e.g. from the full qcow2 URL to `fedora:44`). Subsequent phases like "Install existing and invalid packages" then saw the wrong value, causing `is_image_mode` to return false and the test to check the wrong error message assertion. Assisted-by: Claude Code Signed-off-by: Miroslav Vadkerti <mvadkert@redhat.com>
Signed-off-by: Miroslav Vadkerti <mvadkert@redhat.com>
b9094a5 to
df003bb
Compare
Signed-off-by: Miroslav Vadkerti <mvadkert@redhat.com>
df003bb to
25cf565
Compare
7ee825e to
d34ce84
Compare
Signed-off-by: Miroslav Vadkerti <mvadkert@redhat.com>
867703a to
c0a3c89
Compare
Signed-off-by: Miroslav Vadkerti <mvadkert@redhat.com>
Move package installations from Ansible playbooks to Python code using `guest.package_manager.install()` which works on both regular and image mode (bootc) guests. Playbooks now only handle `/etc` and `/var` mutations. Add `distro_id` and `distro_major_version` facts to `GuestFacts`. Fix FIPS `is_ostree` guard to allow image mode guests. Fixes #4625 Assisted-by: Claude Code Signed-off-by: Miroslav Vadkerti <mvadkert@redhat.com>
Signed-off-by: Miroslav Vadkerti <mvadkert@redhat.com>
Signed-off-by: Miroslav Vadkerti <mvadkert@redhat.com>
Signed-off-by: Miroslav Vadkerti <mvadkert@redhat.com>
d416528 to
e4dc3b1
Compare
Signed-off-by: Miroslav Vadkerti <mvadkert@redhat.com>
| if version == 9: | ||
| guest.package_manager.install(Package("epel-next-release")) |
There was a problem hiding this comment.
This is wrong. epel-next is only for centos
| if version == 7: | ||
| guest.package_manager.install(Package("yum-utils")) | ||
| else: | ||
| guest.package_manager.install(Package("dnf-command(config-manager)")) |
There was a problem hiding this comment.
This should be handled similarly with copr_plugin. We also have a handling of _base_debuginfo_command that can be generalized or implemented in a similar way
Move package installations from Ansible playbooks to Python code using
guest.package_manager.install()which works on both regular and image mode (bootc) guests. Playbooks now only handle/etcand/varmutations.Add
distro_idanddistro_major_versionfacts toGuestFacts.Fix FIPS
is_ostreeguard to allow image mode guests.Fixes #4625
Depends on #4719 which adds testing support for image mode.
Assisted-by: Claude Code
Pull Request Checklist